Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Doc] better documention #747

Merged
merged 55 commits into from
Apr 14, 2020
Merged

[Doc] better documention #747

merged 55 commits into from
Apr 14, 2020

Conversation

archibate
Copy link
Collaborator

@archibate archibate requested a review from yuanming-hu April 11, 2020 10:04
@archibate archibate requested a review from k-ye April 11, 2020 13:01
archibate
archibate commented Apr 11, 2020
View reviewed changes
docs/vector.rst Show resolved Hide resolved
docs/atomic.rst Outdated Show resolved Hide resolved
docs/utilities.rst Outdated Show resolved Hide resolved
k-ye
k-ye reviewed Apr 11, 2020
View reviewed changes
Copy link
Member

@k-ye k-ye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! FYI i think it's better to include native speakers as reviewer.. Again, better to break this down into multiple PRs, otherwise it's hard to do exhaustive reviews

docs/atomic.rst Outdated Show resolved Hide resolved
docs/atomic.rst Outdated Show resolved Hide resolved
docs/atomic.rst Outdated Show resolved Hide resolved
docs/atomic.rst Outdated Show resolved Hide resolved
docs/atomic.rst Outdated Show resolved Hide resolved
docs/cpp_style.rst Outdated Show resolved Hide resolved
docs/vector.rst Outdated Show resolved Hide resolved
docs/vector.rst Outdated Show resolved Hide resolved
docs/vector.rst Outdated Show resolved Hide resolved
docs/vector.rst Outdated Show resolved Hide resolved
@archibate archibate requested a review from k-ye April 11, 2020 14:02
@archibate
Copy link
Collaborator Author

...native speakers as reviewer...

Any native speaker in @taichi-dev? Maybe we can ask @yuanming-hu for help?

@yuanming-hu
Copy link
Member

Let me take a pass first.

yuanming-hu
yuanming-hu reviewed Apr 11, 2020
View reviewed changes
docs/vector.rst Outdated Show resolved Hide resolved
docs/atomic.rst Outdated Show resolved Hide resolved
@yuanming-hu
Copy link
Member

yuanming-hu commented Apr 11, 2020 •
edited
Loading

One more high-level thing: should we use in-place documentation? @archibate @k-ye
https://pythonhosted.org/an_example_pypi_project/sphinx.html#function-definitions

This will allow us to write documentation together with code.

I think we can have two parts: tutorials and a more complete API reference. Tutorials are written by human and API reference generated automatically.

@archibate
Copy link
Collaborator Author

One more high-level thing: should we use in-place documentation? @archibate @k-ye
https://pythonhosted.org/an_example_pypi_project/sphinx.html#function-definitions

This will allow us to write documentation together with code.

I think we can have two parts: tutoriala and a more complete API reference. Tutorials are written by human and API reference generated automatically.

Sounds like a doxygen will do? But I still want to keep some human-written reference for things like ti.Vector([x, y, ...]), which could be hard if generated by machine(?)

@yuanming-hu
Copy link
Member

Sounds like a doxygen will do?

Right, we use sphinx here for the Python part.

But I still want to keep some human-written reference for things like ti.Vector([x, y, ...]), which could be hard if generated by machine(?)

I agree - that's why I believe we should have both tutorials and API references.

@archibate
Copy link
Collaborator Author

Fine, ;et's keep some critical parts to be human-written, like the definition of tensor&vectors. And non-critical parts like the API of GUI, we can machine gen it.

@archibate archibate requested a review from yuanming-hu April 11, 2020 14:55
archibate
archibate commented Apr 11, 2020
View reviewed changes
docs/type.rst Outdated Show resolved Hide resolved
archibate
archibate commented Apr 11, 2020
View reviewed changes
docs/atomic.rst Outdated Show resolved Hide resolved
@archibate archibate mentioned this pull request Apr 12, 2020
Closed
yuanming-hu
yuanming-hu reviewed Apr 14, 2020
View reviewed changes
Copy link
Member

@yuanming-hu yuanming-hu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating the documentation. This is really helpful. I'll take a pass now to fix minor writing issues.

examples/lists.py Outdated Show resolved Hide resolved
@yuanming-hu
Copy link
Member

yuanming-hu commented Apr 14, 2020 •
edited
Loading

Before I do my pass, do you mind doing me a favor and fix these Sphinx warnings? No rush on this. Thanks!

/home/yuanming/repos/taichi/docs/atomic.rst:8: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/atomic.rst:15: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/atomic.rst:27: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/atomic.rst:34: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/atomic.rst:41: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/atomic.rst:60: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/atomic.rst:75: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/contributor_guide.rst:80: WARNING: Inline interpreted text or phrase reference start-string without end-string.
/home/yuanming/repos/taichi/docs/contributor_guide.rst:80: WARNING: Inline interpreted text or phrase reference start-string without end-string.
/home/yuanming/repos/taichi/docs/hello.rst:96: WARNING: Title underline too short.

(Sparse) Tensors
-------
/home/yuanming/repos/taichi/docs/hello.rst:96: WARNING: Title underline too short.

(Sparse) Tensors
-------
/home/yuanming/repos/taichi/docs/scalar_tensor.rst:17: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/scalar_tensor.rst:21: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/scalar_tensor.rst:25: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/vector.rst:24: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/vector.rst:42: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/vector.rst:56: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/vector.rst:60: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/vector.rst:76: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/vector.rst:81: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/vector.rst:96: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/vector.rst:109: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/vector.rst:122: WARNING: Unexpected indentation.
/home/yuanming/repos/taichi/docs/vector.rst:138: WARNING: Unexpected indentation.

@archibate
Copy link
Collaborator Author

Before I do my pass, do you mind doing me a favor and fix these Sphinx warnings? No rush on this. Thanks!

No problem! Fixed all WARNINGs except for contributor_guideline.rst, how to fix that? Seems related to the link.

@yuanming-hu
Copy link
Member

Thanks! I'm taking a pass. Adding a space after _ seems to fix the remaining warning.

yuanming-hu
yuanming-hu reviewed Apr 14, 2020
View reviewed changes
Copy link
Member

@yuanming-hu yuanming-hu left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good in general! I took a pass through atomic, contribution guide, and syntax. I also left a few minor places for you to fix. I'll take a pass through vector and scalar_tensor tomorrow.

A few tips to improve your writing:

  • Proofread before posting anything. I usually read carefully what I typed 3-5 times before I click post or submit
  • Pay attention to details, such as a, the, punctuation and low-level formats
  • Be calm, be positive, and be professional
  • Use a spellchecker (I use Grammarly)

docs/dev_install.rst Outdated Show resolved Hide resolved
docs/dev_install.rst Outdated Show resolved Hide resolved
docs/dev_install.rst Outdated Show resolved Hide resolved
docs/dev_install.rst Outdated Show resolved Hide resolved
docs/global_settings.rst Outdated Show resolved Hide resolved
docs/hello.rst Outdated Show resolved Hide resolved
docs/hello.rst Outdated Show resolved Hide resolved
docs/hello.rst Outdated Show resolved Hide resolved
docs/hello.rst Show resolved Hide resolved
docs/linalg.rst Outdated Show resolved Hide resolved
@archibate archibate mentioned this pull request Apr 14, 2020
Closed
@archibate archibate requested a review from yuanming-hu April 14, 2020 04:59
@archibate
Copy link
Collaborator Author

Travis can't download openssl? Excuse me?

@yuanming-hu
Copy link
Member

Finalized the remaining two files. Thanks for contributing to the documentation!

It's hard to make perfect, but hopefully, readers will catch and fix our errors later.

@yuanming-hu yuanming-hu merged commit db8d2d6 into taichi-dev:master Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@k-ye k-ye Awaiting requested review from k-ye

@yuanming-hu yuanming-hu Awaiting requested review from yuanming-hu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@archibate @yuanming-hu @k-ye @taichi-gardener